initial mosaics cli + async client #1131
Conversation
ischneider
force-pushed
the
mosaics-api
branch
2 times, most recently
from
fbd75a4 to
8b4530a
Compare
|
Thanks a ton for this!! Minor doc comment that could be addressed later: It's a bit unclear from the CLI --help docs that With that said, that's a very minor nitpick that could be handled via more examples or other documentation. I know you separately mentioned it for the python client side, but honestly it's the CLI docs that I think it's the least clear in currently. |
planet/clients/mosaics.py
Outdated
|
|
||
| async def download_quad(self, | ||
| quad: dict, | ||
| directory, |
There was a problem hiding this comment.
In the Data API client, the directory arg defaults to current dir. Is it worth making that consistent? I also think your idea of defaulting to the mosaic name would work
There was a problem hiding this comment.
i changed the default for directory to current for the client - the quad object doesn't have any direct reference to the mosaic name but I did make this the behavior from the CLI
| planet mosaics contributions 09462e5a-2af0-4de3-a710-e9010d8d4e58 455-1273 | ||
|
|
||
| echo -e "\nDownload Them!" | ||
| planet mosaics download 09462e5a-2af0-4de3-a710-e9010d8d4e58 --bbox=-100,40,-100,40.1 --output-dir=quads |
There was a problem hiding this comment.
Just curious if we think users might need guard rails for this. Could this be a significant enough amount of data to warrant a confirmation step?
There was a problem hiding this comment.
Yes, this was one of the issues (1) I raised in the MR description.
|
Thanks @joferkington
Yes, good call to make this clearer. Initially I tried to add As for the examples, I did not consider that interpretation - will make that clearer! |
ischneider
force-pushed
the
mosaics-api
branch
2 times, most recently
from
45bedc9 to
d3b88b7
Compare
- make it clearer that name or ID can be used to lookup items - add typed dicts to represent API responses/resources - accept typed dict or name/ID when possible - raise MissingResource when attempting a get by name with no match - use positional/keyword-only parameters in client methods
ischneider
force-pushed
the
mosaics-api
branch
2 times, most recently
from
4807f06 to
e496b48
Compare
WIP - mosaics API support
I based this on the V1 CLI/SDK but added some additional functionality, mostly in filters that have been added since the original client was created.
Rather than implement the sync SDK and fully document every parameter and write every test, I decided to get this out for some tire kicking first... so there might be a broken thing or two but pretty sure everything is working
The
examples/mosaics-cli.shscript demonstrates some basic functionality and will download 2 quads to the directory it's run from.General questions that popped up while developing:
Some operations do too much (without filters), e.g. list mosaics (for Planet users is 1000s of results), list/download quads for large (or default) extents
Download operations have an overwrite flag but this requires checking the content-disposition header to get the destination file name (at least with the current core implementation) though this will result in the request counting as a "download" (for quota) even if the stream is never read...
Some operations accept a name or ID (as a convenience, since ID is opaque) and others require a dict of the the resource representation - it would be nice to allow any of the above, i.e.
NameIDObject = Union[dict, str](this name is terrible but what should it be?)Proposals (where I have an idea):
For requests that do too much: limit can be confusing when there's a default and users may not be aware of it. Instead, the CLI command can fail once too many results are processed. For quads, could use a heuristic based on quads/square-degree (e.g. the 10 degree bounding box you are supplying will download approximately 4733 quads) and prompt the user?
The v0 basemaps API used a primary quad identifier that contained the zoom and tile coordinates in northing and easting, left padded with zeros, e.g.
L15-0450E-1275N.tif. Since the v1 API only allows access to native resolution (you cannot get L14), and the padding is terribly useful, propose changing the default name to simply the quad-id (450-1275 in this case) with the option of auto-creating a directory with the name of the mosaic. Or, more forward thinking, allow choosing one of several built-in output formats. This would require modification ofplanet.models.StreamingBody(https://github.com/planetlabs/planet-client-python/blob/main/planet/models.py#L82) to allow specifying the output name.Related Issue(s):
Closes #446
Proposed Changes:
For inclusion in changelog (if applicable):
PR Checklist:
(Optional) @mentions for Notifications: